feat(web): add safe persisted session deletion - #396
Conversation
tt-a1i
left a comment
tt-a1i
left a comment
There was a problem hiding this comment.
Exact-head review: persisted Session deletion is valuable, but two blockers remain. Focused adapter/host tests passed 42/42. An additional probe using the real PiWebRuntime activation/retention methods, real Pi SessionManager files, and PiWebAdapter reproduced deletion of a still-streaming background Session and loss of its original history. The fake agent lifecycle seam follows existing runtime tests; no provider call or installed UI acceptance is claimed. The destructive endpoint also omits the native reviewed confirmation required by #347. No source changes or merge performed.
|
Fixed and pushed as
|
tt-a1i
left a comment
tt-a1i
left a comment
There was a problem hiding this comment.
Reviewed exact head 7e16465.
This backend deletion primitive is valuable, and the retained-runtime guard is an improvement, but two P1 lifecycle blockers remain:
- [P1] The live-Session ownership check and unlink are not atomic. deleteSession() samples isSessionOwned(...) and later calls rm(), while Session selection/creation is serialized through a different controller-mutation boundary. A concurrent select can acquire the target after the check and before unlink, so a file can become live and then be deleted. Admission and deletion need to share the authoritative runtime lifecycle boundary, with an interleaving test.
- [P1] confirm= is only caller-supplied path echo, not the native reviewed confirmation required by #347. Any authenticated API caller can send it in the same request; there is no independently reviewed, fresh, one-shot grant.
There are also P2 failure-semantics gaps: the file is removed before archive/workspace metadata updates, so a later write failure can return 500 after irreversible deletion without publishing session_deleted; missing/corrupt/repeated-delete cases also collapse to generic 500 and remain untested.
Required CI is green, but these are runtime safety invariants, so this should not merge yet.
2 participants
Problem
Related to #347. Web Workbench can archive Sessions but cannot safely remove a persisted non-active Session. Deletion must not be confused with archive metadata removal, and the active Session must never be deleted.
Value
Adds a bounded, auditable persistence-management primitive for Session retention while keeping Pi JSONL files authoritative and preventing accidental active-session loss.
Approach
DELETE /api/sessions?path=...endpoint..jsonlfile inside the configured Web Session directory.409 SESSION_CONFLICTresponse.session_deletedevent for connected clients.Validation
biome format/biome lint --error-on-warnings: passed.tsc --noEmit: passed.bunis not installed in this environment, so the equivalent repository scripts were run with the bundled Node 24 executable and local Biome/Vitest binaries.Impact